fix(auth): harden existing WebAuthn ceremonies - #2849

I have a few concerns about scope, deployment documentation, and the existing WebAuthn settings.

First, could the FRONTEND_DIST_DIR change be moved to a separate PR?

This appears to be a general build feature rather than something required specifically for passkeys. It allows build.sh to replace the bundled frontend with the contents of a local directory, which could be useful for many unrelated backend/frontend changes.

It also changes the trust boundary of the build process: when enabled, arbitrary files from the supplied directory may be included in the final backend artifact. Please document the intended use of this option and confirm that release workflows cannot set it from untrusted pull-request input, downloaded artifacts, or uncontrolled external paths.

Keeping this change separate would make both the passkey implementation and the build/release security impact easier to review.

Second, the PR describes several deployment constraints that should be documented before the feature is merged:

• production requires an absolute HTTPS site_url;
• the configured RP ID and exact origin are enforced;
• authentication challenges are currently stored in memory;
• multi-replica deployments therefore require a shared store with atomic consume semantics.

These details can directly affect registration and login behind reverse proxies or load balancers, especially when requests are routed between instances. Is there a corresponding documentation PR, or could the required documentation be added as part of this work?

Finally, please clarify how this interacts with the existing WebAuthn login setting.

In particular:

• Does enabling the existing setting automatically make passkey login available?
• What happens to WebAuthn credentials that were created before this change?
• Can users still view, add, or remove registered passkeys when login through this method is disabled?
• Is any migration or administrator action required?

The PR mentions backward-compatible credential decoding, which addresses the storage format, but the user-visible behavior and migration expectations should also be documented.

Addressed in 02ab2b2 and the coordinated documentation PR OpenListTeam/OpenList-Docs#348:

• Removed the FRONTEND_DIST_DIR build change from this PR; build.sh now matches upstream main.
• Documented the absolute HTTPS site_url, exact origin/RP validation, reverse-proxy headers, five-minute in-memory challenges, single-process topology, and shared atomic-store requirement for multiple replicas.
• Clarified setting behavior: webauthn_login_enabled gates both login and management ceremonies; disabling it does not delete stored credentials, and re-enabling restores them.
• Existing WebAuthn credentials are decoded compatibly; no database migration, re-registration, or administrator action is required. Older credentials receive generated display names/unknown creation metadata until renamed, and last_used_at is populated after a successful assertion.

Evidence: focused backend tests pass (go test ./internal/authn ./internal/db ./server/handles), docs formatting passes, and the docs SSG build passes.

Final Oracle ARM64 verification for head 02ab2b2c:

• go test -p=1 ./internal/authn ./internal/db ./server/handles passed from the exact PR source.
• go test -p=1 ./... completed. Passkey-related packages passed; the remaining failures are pre-existing/environmental: non-constant format-string vet findings in unrelated drivers, missing fuse.h, tests requiring a local aria2 service on localhost:6800, and the existing internal/net proxy transport assertion.
• A source-stamped Linux/arm64 image built successfully with frontend head e39da3e.
• Chromium over public HTTPS completed password login → discoverable passkey registration → passkey login → replay rejection (code=400) → metadata/rename → revoke → revoked-credential rejection → password-login regression. Console errors and unexpected network failures: zero.

GitHub's Test Build and Beta Release (Docker) runs remain action_required with zero jobs because fork workflows require a repository maintainer's approval; there is no failed CI job to address yet.

I rechecked the current head and the resolved P1 thread. The change mitigates the original failure mode, but I do not think the unauthenticated challenge-store DoS has been fully addressed.

At the 10,000-entry ceiling, every public begin-login request acquires the global lock, scans the entire challenge map, and evicts the oldest live challenge. Sustained unauthenticated traffic can therefore:

• repeatedly trigger O(n) scans while holding the store lock;
• churn or evict legitimate pending ceremonies;
• consume CPU without needing a valid account.

Evicting the oldest live challenge prevents the store from rejecting every new insertion, but it does not provide admission control and can replace the original lockout with challenge churn and a CPU-heavy hot path.

Please add rate or admission controls that cannot be monopolized by anonymous traffic, avoid an O(n) scan on every insertion at capacity, and add a regression test that demonstrates a legitimate ceremony can still complete during sustained hostile begin-login traffic.

Please also remove the feature-specific changes to the repository-level agent instructions. Those instructions are not part of the passkey implementation or operator documentation and make the PR scope harder to review.

After these changes, this PR and the coordinated frontend PR should receive another security review before merge.

After reviewing the updated scope, I suggest considering a split between the existing WebAuthn security hardening and the broader passkey product changes.

The current PR contains two categories of work with different review and maintenance requirements:

1. Security hardening for the existing WebAuthn implementation, including server-side one-time challenges, ceremony and user binding, replay protection, exact origin/RP validation, required user verification, and assertion-counter persistence.

2. Broader passkey functionality, including named credentials, additional metadata, rename and management behavior, expanded frontend flows, visual assets, and user/operator documentation.

Separating them could make the security-sensitive portion easier to audit and allow it to proceed independently of the product decision around the expanded passkey experience. It would also provide a clearer rollback boundary and keep the coordinated frontend and documentation changes attached to the feature portion.

A possible split would be:

• a minimal backend security PR preserving the current WebAuthn behavior and credential compatibility, including resolution of the remaining challenge-store concern;
• a separate feature PR for named credentials, metadata, management APIs, frontend UX, assets, and documentation.

This is a design suggestion rather than a request to split at this stage. Before making further changes, please provide your own assessment of:

• whether this split is technically practical;
• which backend changes must remain together and why;
• what coupling would remain between the two parts;
• whether you personally intend to maintain the resulting authentication behavior.

The recent responses across the coordinated PRs are highly uniform and read more like generated status reports than a design discussion. Under the repository's automated-contribution policy, another automated acknowledgement or implementation summary is not sufficient. Please confirm that you have personally reviewed the implementation and explain the trade-offs of splitting it in your own words.

The security PR keeps the changes that must be audited together: server-side one-time challenges, admission control, ceremony/user binding, exact origin and RP validation, required user verification, replay protection, and assertion-counter persistence. These all belong to the existing WebAuthn authentication path and should remain independent of new product features.

The second PR contains the optional product layer: credential names, created/last-used metadata, rename and management APIs, frontend UX, and the corresponding user documentation. Its only intentional dependency is on the hardened backend behavior from the first PR.

This split gives the security work a smaller audit and rollback boundary, while allowing the broader Passkey experience to be reviewed as a separate product decision. I personally intend to maintain both the security behavior and the Passkey management functionality after merge.

@ILoveScratch2 The requested changes have been addressed in the current security-only head e727a925: the feature-specific AGENTS.md change and non-standard comments were removed, anonymous challenge admission is bounded, and expiry handling no longer scans the full challenge map. The broader Passkey management functionality is now split into #2861. All inline review threads are resolved, and the focused Oracle tests and full backend build pass. Could you please re-review #2849 when available?